Confirm before deleting a source; --yes skips for scripts - #139

Independent code review trail

Ran the independent reviewer (Opus, fresh context, diff on stdin) across four passes; every finding was addressed. Summary per round:

Round 1

• Scripts calling sources delete <uuid> now hang / silently no-op on non-TTY stdin — fixed: added a TTY guard that fails loud and points at --yes instead of letting inquirer's stdin-EOF abort be swallowed as Ctrl+C.
• --yes with no uuid still opens the picker (blocks scripts) — fixed: --yes now requires an explicit uuid.
• Missing tests for abort paths — added.
• Confirm named the uuid only after the user picked by name — fixed: thread the source name into the prompt.

Round 2

• Direct-uuid confirm only echoed the typed string (no protection against a wrong-but-valid copy-pasted uuid) — fixed: look the source up (best-effort, like sources update) to name it.
• --yes create/update test asserted only the exit code, not that dispatch was skipped — fixed: added negative assertions, split per subcommand.
• TTY guard checked stdin but inquirer renders to stdout — fixed: require both stdin and stdout to be TTYs; added a redirected-stdout test.
• Four near-identical failWithUsage guards (rule of three) — fixed: extracted usageErrorFor.
• Dense nested-await in a negated compound condition — fixed: pulled into named steps.

Round 3

• Label lookup could abort a delete on a propagated timeout (fetchSources re-throws ApiTimeoutError); comment was factually wrong — fixed: lookup is now genuinely best-effort (.catch), falls back to the bare uuid.
• A lookup miss was indistinguishable from a hit — fixed: the miss is called out in the label.
• Third copy of fetch+find (rule of three) — fixed: extracted lookupSourceByUuid, shared by findSourceByUuid and the delete label.
• usageErrorFor read process globals despite its parameterized signature — fixed: isInteractive is now passed in.
• Added a test asserting --yes skips the label lookup's fetch entirely.

Round 4

• A failed load (outage → fetchSources returns [], or a thrown timeout) was reported as "no source found with this uuid" — false during an outage — fixed: three outcomes are now distinct (hit / loaded-but-no-match → "no matching source in the source list" / lookup threw → "source name unavailable — could not load the list"); undefined marks a thrown lookup, null a loaded miss.
• Timeout test passed on the broken message — fixed: added not.toContain the miss note plus a positive assert on the failure note.
• Declined-confirmation test didn't assert a clean (non-error) exit — fixed.
• confirmSourceDeletion wrapper comment overclaimed that the extraction enables the --yes short-circuit — the || at the call site is what short-circuits; kept the wrapper for call-site readability (and to avoid the nested-await round 2 flagged) and corrected the comment.

Skipped (intentional, with reason)

• Widen the TTY guard to create/update (raised round 3): out of scope for Confirm before sources delete <uuid> #135. Those commands prompting without a TTY predates this change; guarding only the irreversible delete is the task. Documented inline.

Final state: lint, typecheck, build, and the full suite (722 tests) all green. No unresolved findings.

Merge + code-review trail (agent)

Merged origin/main into this branch. Conflicts were present in 3 files, all from main's newly-added rotate-secret subcommand overlapping this PR's delete-confirmation change:

• src/commands/sources.ts — combined the @inquirer/prompts import (confirm + password), the USAGE text (delete's confirm wording + the rotate-secret line), and the handler map (both entries).
• README.md — merged the sources-command row so it lists delete…--yes and rotate-secret.
• tests/commands/sources.test.ts — collapsed the two duplicate vi.mock('@inquirer/prompts') blocks into one mocking input, password, select, confirm.

No work discarded from either side. After resolving: lint clean, typecheck clean, 795 tests pass, build passes.

Independent review loop (Opus, 3 rounds)

Round 1 — fixed:

• deleteSourceCommand: targetUuid = uuid ?? picked?.uuid used nullish coalescing while the line above branched on truthiness, so delete "" opened the picker then silently discarded the pick on the !targetUuid guard. Changed to uuid || picked?.uuid; added a test.
• NO_MATCH_NOTE overclaimed a confirmed non-match when fetchSources() had swallowed a transport error into []. Reworded to no matching source found, or the list could not be loaded (matching findSourceByUuid); updated the two assertions.

Round 2 — clean: the one finding (NO_MATCH ambiguity) was already resolved by the round-1 reword; reviewer confirmed everything else checks out.

Round 3 — fixed:

• Failed delete used a bare console.error, so a scripted delete <uuid> --yes || notify read a failed delete as success (exit 0). Switched to failWithMessage(...) (exit 1), matching rotateSecretForSource's pattern; test now asserts process.exitCode === 1.
• Non-TTY delete error advised --yes without mentioning it needs a uuid, causing a second different error on the next attempt. Message now shows markpost sources delete <uuid> --yes.

Skipped (out of scope for a merge-resolution PR): style/comment-volume findings on already-reviewed feature code (inline the confirmSourceDeletion wrapper, trim comments) — these re-litigate deliberate decisions from this PR's earlier 4 review rounds, not defects.